Conversation
There was a problem hiding this comment.
Pull request overview
Updates the repo to a newer libdatadog revision and adapts serverless trace/stats flushing code and dependency pins to remain compatible with the upstream API and constraints.
Changes:
- Bump
libdd-*git dependencies to rev986aab55…and addlibdd-capabilitieswhere required. - Adapt
trace_flusher/stats_flusherto the newHttpClientTrait-basedSendData::sendAPI (including a local proxy-capable client wrapper). - Pin
rustls-native-certsto<0.8.3, update cloud env detection tests, and refresh lockfile/license inventory entries.
Reviewed changes
Copilot reviewed 8 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| crates/datadog-trace-agent/src/trace_flusher.rs | Introduces ProxyHttpClient implementing HttpClientTrait and updates trace flushing to use it |
| crates/datadog-trace-agent/src/stats_flusher.rs | Switches stats sending to use libdd_common::DefaultHttpClient |
| crates/datadog-trace-agent/src/config.rs | Updates tests to include FUNCTION_TARGET for Cloud Run Function detection |
| crates/datadog-trace-agent/Cargo.toml | Bumps libdatadog revs and adds libdd-capabilities dependency |
| crates/datadog-serverless-compat/Cargo.toml | Bumps libdd-trace-utils rev |
| crates/datadog-fips/Cargo.toml | Pins rustls-native-certs to <0.8.3 |
| crates/datadog-agent-config/Cargo.toml | Bumps libdatadog revs |
| LICENSE-3rdparty.csv | Adds libdd-capabilities* entries and updates openssl-probe repo URL |
| Cargo.lock | Updates resolved dependency graph for the new rev and TLS-related pins |
| Err(e) => { | ||
| error!("Failed to create HTTP client: {e:?}"); |
There was a problem hiding this comment.
The error log here formats the underlying error with {:?}. Since with_proxy() parses the proxy URL, the error value may include the raw proxy URL (including potential userinfo credentials), which would leak secrets into logs. Consider logging a sanitized/redacted proxy URL (strip userinfo) or logging a higher-level error without embedding the raw URL/error debug that might contain it.
| Err(e) => { | |
| error!("Failed to create HTTP client: {e:?}"); | |
| Err(_e) => { | |
| error!("Failed to create HTTP client from configured proxy URL"); |
There was a problem hiding this comment.
This was the pre-existing behavior
| #[allow(clippy::expect_used)] | ||
| fn new_client() -> Self { | ||
| Self::with_proxy(None).expect("building proxy connector with default TLS should not fail") | ||
| } |
There was a problem hiding this comment.
HttpClientTrait::new_client uses expect(...) and will panic if building the default proxy connector fails (e.g., TLS/init issues). Even if it’s not currently called, it’s part of the trait contract and could be exercised by upstream code in the future. Consider making new_client panic-free (e.g., store the construction error in the struct and have request() return HttpError), or otherwise ensure failures are handled without crashing the process.
There was a problem hiding this comment.
ProxyHttpClient is a private struct, I think it'd be very unlikely for new_client to be exercised by upstream code in the future
What does this PR do?
rustls-native-certsto <0.8.3 in datadog-fips to match the new libdd-common constraint (libdd-common wants <0.8.3, datadog-fips was on ^0.8.1 which resolved to 0.8.3)stats_flusherandtrace_flusherto the newHttpClientTrait-basedSendData::sendAPI and addslibdd-capabilitiesas a direct dependency ofdatadog-trace-agent. Needed as a result of feat(capabilities)!: trait architecture http libdatadog#1555Motivation
https://datadoghq.atlassian.net/browse/SVLS-8799
DataDog/libdatadog#1857 updates the agent's cloud environment detection logic to enable customers instrumenting their Azure Functions to set the
FUNCTION_NAMEenvironment variable. Previously,FUNCTION_NAMEwas used to identify a Google Cloud Run Function; this change makes the detection logic more specific by checking multiple cloud-specific env vars at once.Additional Notes
SendData::sendis now generic overH: HttpClientTraitinstead ofGenericHttpClient<C: Connect>, so our flushers no longer compiled against the new rev.stats_flusher: now useslibdd_common::DefaultHttpClienttrace_flusher: needs proxy support whichDefaultHttpClientdoesn't provide. Introduced a localProxyHttpClientnewtype that wrapsGenericHttpClient<ProxyConnector<Connector>>and implementsHttpClientTraitrequest()body mirrors libdatadog's ownDefaultHttpClient::requestFUNCTION_TARGETalongside everyKSERVICEintest_default_trace_and_trace_stats_urlssinceread_cloud_envin libdatadog now requires both env vars to detect a Cloud Run Function when creating a ConfigDescribe how to test/QA your changes
Deployed an Azure function and set the env var
FUNCTION_NAME. Confirmed that traces continued to flow to Datadog